Updates to Bootstrap 5 and adds theme support for code coverage.#1036
Updates to Bootstrap 5 and adds theme support for code coverage.#1036gammamatrix wants to merge 5 commits intosebastianbergmann:mainfrom
Conversation
…pdate for test EndToEndTest::testPathCoverageForBankAccountTest
|
|
||
| </tbody> | ||
| </table> | ||
| %a |
There was a problem hiding this comment.
Not sure if this is correct way to fix this test by adding the footer.
… for BankAccount.php_path.html.
|
Should I create another Pull Request, in PHPUnit, to add a theme variable to phpunit.xsd? <xs:complexType name="coverageReportHtmlType">
<xs:attribute name="outputDirectory" type="xs:anyURI" use="required"/>
<xs:attribute name="lowUpperBound" type="xs:integer" default="50"/>
<xs:attribute name="highLowerBound" type="xs:integer" default="90"/>
<xs:attribute name="colorSuccessLow" type="xs:string" default="#dff0d8"/>
<xs:attribute name="colorSuccessMedium" type="xs:string" default="#c3e3b5"/>
<xs:attribute name="colorSuccessHigh" type="xs:string" default="#99cb84"/>
<xs:attribute name="colorWarning" type="xs:string" default="#fcf8e3"/>
<xs:attribute name="colorDanger" type="xs:string" default="#f2dede"/>
<xs:attribute name="customCssFile" type="xs:string"/>
<xs:attribute name="theme" type="xs:string" default=""/>
</xs:complexType>The colors for low, medium, high, warning, and danger can be controlled by setting the CSS Variables: .table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: {{success-low}};
/* background-color: rgb(from var(--bs-success) r g b / 0.25); */
}
The Bootstrap CSS Variables may be overridden in a theme defined with the existing: NOTE: The CSS rules defined in .table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: var(--bs-teal);
}I tested passing both overriding with the customCssFile and in overriding the colors with CSS variables in the phpunit.xml configuration as well and both work fine.
colors<html
outputDirectory="output/html"
lowUpperBound="50"
highLowerBound="90"
customCssFile="./phpunit-themes.css"
colorSuccessLow="rgb(from var(--bs-purple) r g b / 0.75)"
colorSuccessMedium="rgb(from var(--bs-blue) r g b / 0.55)"
colorSuccessHigh="rgb(from var(--bs-teal) r g b / 0.25)"
colorWarning="rgb(from var(--bs-warning) r g b / 0.25)"
colorDanger="rgb(from var(--bs-danger) r g b / 0.25)"
theme="dark" />And with these custom CSS rules:.table tbody tr.covered-by-medium-tests td, li.covered-by-medium-tests {
background-color: purple;
}
.table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: aquamarine;
}
.table tbody tr.danger td, .table tbody td.danger, li.danger, span.danger {
background-color: wheat;
}
.table tbody tr.covered-by-large-tests td, li.covered-by-large-tests, tr.success, td.success, li.success, span.success {
background-color: aquamarine;
}
.table tbody tr.covered-by-medium-tests td, li.covered-by-medium-tests {
background-color: purple;
}
.table tbody tr.covered-by-small-tests td, li.covered-by-small-tests {
background-color: var(--bs-teal);
}
.table tbody tr.warning td, .table tbody td.warning, li.warning, span.warning {
background-color: var(--bs-yellow);
}
.table tbody tr.danger td, .table tbody td.danger, li.danger, span.danger {
background-color: crimson;
}
.covered-by-small-tests, tr.covered-by-small-tests td {
background-color: var(--bs-teal);
}
.covered-by-medium-tests, tr.covered-by-medium-tests td {
background-color: purple;
}
.covered-by-large-tests, tr.covered-by-large-tests td {
background-color: aquamarine;
}
.not-covered, tr.not-covered td {
background-color: crimson;
}
.not-coverable, tr.not-coverable td {
background-color: var(--bs-yellow);
} |
|
I wonder whether we need this new |
The main reason why the <html
outputDirectory="output/html"
lowUpperBound="50"
highLowerBound="90"
theme="dark" />
I added some more information on the changes needed for the |
|
Please split this into two pull requests: one for the Bootstrap update and one for the functional changes. |
Here are the Bootstrap-only changes, for FYI:
final class Colors
{
private readonly string $successLow;
private readonly string $successMedium;
private readonly string $successHigh;
private readonly string $warning;
private readonly string $danger;
public static function default(): self
{
return new self(
'rgb(from var(--bs-success) r g b / 0.1)',
'rgb(from var(--bs-success) r g b / 0.33)',
'rgb(from var(--bs-success) r g b / 0.67)',
'rgb(from var(--bs-warning) r g b / 0.1)',
'rgb(from var(--bs-danger) r g b / 0.1)',
);
}
<html lang="en" data-bs-theme="dark"> |
|
FYI: I just tried running the tests, my installation is on PHPUnit 11.3.0 and I did not see the whitespace error.
|
|
My last test (#1037 (comment)) was with the other PR (#1037). Can you please test with that, too? Thanks! |
|
@sebastianbergmann Should we close this PR?
|
|
Support for light/dark mode was implemented in #1095. |
Changes
Assets
I used these from jsdeliver and Cloudflare:
Theme support
Bootstrap 5.3 comes with dark and light mode support.
The
<html>tag of each top level webpage gets a new element:themevariable in code coverage section:For example: src/Report/Html/Renderer/Template/directory.html.dist
This custom theme file provides some simple examples, the colors are not perfect, but good enough to test the code.
I put these themes into
customCssFile="./phpunit-themes.css"Example phpunit-themes.css for customCssFile
phpunit.xml changes
Proposed theme variable:
CSS Variables in style.css
I had to slightly adjust the existing CSS rules, such as adding a
tdto increase override priority from the Bootstrap 5 rules.See src/Report/Html/Renderer/Template/css/style.css
These new default rules should be compatible across other themes.
Here are few I updated, but did not finalize:
Dev Notes
Testing themes
Here is a screenshot of testing the example themes in:
phpunit-themes.cssYou can inspect the DOM to manually change
Tech Debt
There are a few other things that are a bit out of date.
NOTE: I think these fixes are beyond the scope of this PR and I would not mind tackling those after this:
align="center".